Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul cleaners plugin; Add Maps::isPlantInBox #4949

Open
wants to merge 36 commits into
base: develop
Choose a base branch
from

Conversation

Bumber64
Copy link
Contributor

@Bumber64 Bumber64 commented Sep 20, 2024

Overhaul cleaners to use cuboid selection instead of operating on entire map only. Can now remove grass (or just unused grass events for performance), skip cleaning blood/vomit/ooze when performing other cleanings, and counts arrow debris removal as affecting block.

Now uses argparse, which will affect existing command syntax. spotclean now just calls the existing map cleaning fn with a 1x1x1 cuboid. It could probably be replaced by an alias, assuming those work with hotkeys.

Removed clean plants option, as this was just rainwater (which evaporates and will be reapplied when it rains).

Moved a plant plugin utility fn out to Maps::isPlantInBox because it was going to be used for plant contaminant removal. This will detect if trees intersect with a box, so someone might have use for it.

Made burrows.cpp use Maps::forCoord instead of a nested x,y,z iterator.

Made tile-material.lua use maps.getPlantAtTile and handle tree root material properly.

Comment on lines 339 to 365
command_result spotclean(color_ostream &out, vector<string> &parameters)
{ // Hotkey command, already suspended
DEBUG(log, out).print("Doing spotclean.\n");
if (!Maps::IsValid())
{
out.printerr("Map is not available.\n");
return CR_FAILURE;
}
df::map_block *block = Maps::getTileBlock(cursor->x, cursor->y, cursor->z);
if (block == NULL)
auto pos = Gui::getCursorPos();
if (!pos.isValid())
{
out.printerr("The keyboard cursor is not active.\n");
return CR_WRONG_USAGE;
}
auto block = Maps::getTileBlock(pos);
if (!block)
{
out.printerr("Invalid map block selected!\n");
return CR_FAILURE;
}
clean_options options;
options.mud = true;
options.snow = true;

for (auto evt : block->block_events)
{
if (evt->getType() != block_square_event_type::material_spatter)
continue;
// type verified - recast to subclass
df::block_square_event_material_spatterst *spatter = (df::block_square_event_material_spatterst *)evt;
clean_mud_safely(spatter, block->map_pos, df::coord(cursor->x % 16, cursor->y % 16, 0));
}
clean_block(out, block, cuboid(pos), options);
return CR_OK;
}
Copy link
Contributor Author

@Bumber64 Bumber64 Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make spotclean an alias for clean here -mds or possibly clean here -ads? (Should it include item spatter? A full command can be used if you want to avoid cleaning tree fruit.)

@Bumber64
Copy link
Contributor Author

Need to properly document syntax changes and clean plant removal.

@Bumber64 Bumber64 force-pushed the cleaners_etc branch 2 times, most recently from 0798023 to 6d26265 Compare December 8, 2024 08:38
Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More review to come, but let me submit the comments I have so I don't leave you hanging

@@ -54,8 +54,10 @@ Template for new versions:
## New Tools

## New Features
- `cleaners`: positional options to limit cleaning to a cuboid area. Options to remove grass or skip blood spatter removal.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `cleaners`: positional options to limit cleaning to a cuboid area. Options to remove grass or skip blood spatter removal.
- `clean`: positional options to limit cleaning to a cuboid area. Options to remove grass or skip blood spatter removal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's an anchor alias that makes clean resolve to cleaners, and everyone knows this tool as "clean". We should really rename the plugin itself to "clean" at some point to reduce confusion..


## Fixes
- ``tile-material.lua``: made root tiles return tree material when not using GetLayerMat.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you phrase this as a user-visible effect on a user-runnable tool?

Copy link
Contributor Author

@Bumber64 Bumber64 Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any actual uses of it. There's one script buried in the Bay12 DFHack thread for regenerating ore veins. milochristiansen mentioned (in the last issue addressing it) that he used it in some old mods, but those aren't in any of his repos.

Users can just use probe.

I guess we should keep the script just in case someone needs it, but remove the entry from the changelog?

docs/changelog.txt Outdated Show resolved Hide resolved
docs/dev/Lua API.rst Show resolved Hide resolved
docs/dev/Lua API.rst Outdated Show resolved Hide resolved
docs/plugins/cleaners.rst Outdated Show resolved Hide resolved
docs/plugins/cleaners.rst Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants